Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporary branch to investigate e2e test failures while upgrading eth-bridge-integration to v0.8.1 #716

Conversation

james-chf
Copy link
Contributor

@james-chf james-chf commented Oct 31, 2022

This PR is based on #704 but targeting eth-bridge-integration so that CI can merge

The last thing before PR #704 can merge is to fix the e2e tests, which I've spent a fair bit of time debugging although am not making too much progress.

Most of the investigation in this PR is happening with the pos_init_validator e2e test.

fn pos_init_validator() -> Result<()> {

It looks like there is an issue where if you create a validator account using some form of namadac init-validator, the ledger at some point later on (i.e. not necessarily immediately) is not able to read it:

2022-10-31T13:11:51.681156Z DEBUG namada::ledger::pos::storage: Reading eth hot key public_key_type="namada::types::key::common::PublicKey" key=Established: atest1v4ehgw36g4pyg3j9x3qnjd3cxgmyz3fk8qcrys3hxdp5xwfnx3zyxsj9xgunxsfjg5u5xvzyzrrqtn
2022-10-31T13:11:51.681265Z DEBUG namada::ledger::storage: storage read key #atest1v9hx7w362pex7mmxyphkvgznw3skkefqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqq8ylv7/validator/#atest1v4ehgw36g4pyg3j9x3qnjd3cxgmyz3fk8qcrys3hxdp5xwfnx3zyxsj9xgunxsfjg5u5xvzyzrrqtn/eth_hot_key
The application panicked (crashed).
Message:  Couldn't decode [48, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 1, 1, 2, 23, 227, 25, 189, 48, 71, 5, 3, 206, 144, 167, 41, 49, 167, 102, 67, 232, 245, 96, 244, 180, 73, 135, 227, 74, 49, 54, 130, 232, 154, 221, 184]: DeserializationError(Custom { kind: InvalidData, error: "Not all bytes read" })
Location: /opt/workspace/heliax/repos/namada/coding/eth-bridge-integration/shared/src/ledger/pos/storage.rs:529

In this case, there is something written to #atest1v9hx7w362pex7mmxyphkvgznw3skkefqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqq8ylv7/validator/#atest1v4ehgw36g4pyg3j9x3qnjd3cxgmyz3fk8qcrys3hxdp5xwfnx3zyxsj9xgunxsfjg5u5xvzyzrrqtn/eth_hot_key, but it is not deserializable to the expected type (types::ValidatorEthKey<Self::PublicKey>). This issue doesn't occur for the genesis validator's eth_hot_key, which deserializes fine, so there may be some mismatch between the write_validator_eth_hot_key used in the tx_init_validator wasm and the native read_validator_eth_hot_key that is part of impl PosBase for Storage<D, H>. The native write_validator_eth_hot_key that is part of impl PosBase for Storage<D, H> seems to work fine.

sug0 and others added 30 commits September 6, 2022 16:20
This inverts the wrapping of errors, storage_api::Error now wraps
NativeVp:Error and some other IBC and PoS custom errors
* namada/tomas/sorted-prefix-iter:
  [ci skip] wasm checksums update
  changelog: add #458
  tests: extend prefix iter tests for reverse order
  add support for rev_iter_prefix in storage and VP and tx envs
  test/vm_host_env: check prefix iter order in tx and VP
  shared/storage/key: add support for int/uint keys that maintain order
  deps: replace hex with data-encoding
  changelog: add #335
  wasm checksums update
  test/vm_host_env: refactor prefix iter tests with new `iter_prefix` fn
  storage_api: build a nicer `iter_prefix` function on top of StorageRead
  changelog: add #331
  update wasm checksums
  fix missing StorageWrite for Storage merkle tree update
  storage: remove unnecessary clone
  add more comments for StorageRead lifetime with an example usage
  add <'iter> lifetime to StorageRead trait to get around lack of GATs
  ledger: impl StorageWrite for Storage
  ledger: factor out TxEnv write methods into a new StorageWrite trait
  update wasm checksums
  ledger/pos: implement PosReadOnly using StorageRead trait
  ledger/tx: inherit StorageRead in TxEnv
  ledger/storage: impl StorageRead for Storage
  ledger/vp: impl StorageRead for WASM VPs pre and post state
  ledger/native_vp: implement StorageRead for pre and post state
  ledger: add StorageRead trait with extensible error type
  [ci skip] wasm checksums update
  update wasm checksums
  wasm_for_tests: make
  Update shared/src/ledger/vp_env.rs
  changelog: add #1093
  wasm: improve error handling
  macros: add error handling to transaction and validity_predicate macros
  wasm: update for VM API changes
  tests: update for VM API changes
  VM: move vm_env sub-mod into tx/vp_prelude and add Tx/VpEnv and Ctx
  shared: update native_vp implementations for VpEnv methods
  shared/ledger/native_vp: implement VpEnv
  shared/vm: rename host_env TxEnv/VpEnv to TxVmEnv/VpVmEnv
  ledger: add tx and VP traits with host env functions
* tomas/vp-tx-env-concrete-error:
  changelog: add #465
  [ci skip] wasm checksums update
  ledger: use storage_api::Error in VpEnv and TxEnv instead of generic
  update wasm checksums
  changelog: add #380
  shared: Add pre/post to VpEnv and use them to provide default impls
  changelog: add #334
  wasm checksums update
  storage_api: add default borsh encoded read/write impl and handle errors
@james-chf james-chf changed the base branch from eth-bridge-integration to interop/ethbridge/merge-0.8.1 October 31, 2022 13:46
@james-chf james-chf changed the base branch from interop/ethbridge/merge-0.8.1 to eth-bridge-integration October 31, 2022 14:39
@james-chf james-chf closed this Oct 31, 2022
@james-chf james-chf deleted the james/ethbridge/tmp/e2e-failures-investigating branch October 31, 2022 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.